-
Notifications
You must be signed in to change notification settings - Fork 20
Use a set of CMakeLists.txt and package.xml files for server and test #61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use a set of CMakeLists.txt and package.xml files for server and test #61
Conversation
Signed-off-by: Barry Xu <[email protected]>
|
I found that making changes will affect the tests. I also need to modify other files. |
Signed-off-by: Barry Xu <[email protected]>
|
Regarding the CI test errors, according to the log, the old script is still being used for execution. |
i believe that we need to update test_launch.py to use new package name which is persist_parameter_server? that is why parameter server is not online during test, see https://github.com/fujitatomoya/ros2_persist_parameter_server/actions/runs/18122360082/job/51569615867?pr=61 |
|
@Mergifyio rebase |
❌ Base branch update has failedGit reported the following error: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR restructures the package organization by consolidating CMakeLists.txt files and renaming the package from "parameter_server" to "persist_parameter_server" to align with ROS distribution requirements.
- Merged separate CMakeLists.txt files from
serverandtestdirectories into a single top-level file - Renamed package from "parameter_server" to "persist_parameter_server" throughout all files
- Added CLIENT_TEST_DEMO CMake option to conditionally build test components
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| CMakeLists.txt | Updated project name and merged server/test build configurations with conditional test building |
| package.xml | Changed package name to persist_parameter_server |
| test/package.xml | Removed separate test package.xml file |
| test/CMakeLists.txt | Removed separate test CMakeLists.txt file |
| Multiple files | Updated package references from parameter_server to persist_parameter_server |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
|
@Barry-Xu-2018 i just merged this PR, thanks! |
I encountered an issue when releasing the persist_parameter_server package.
With the current directory structure, running "catkin_generate_changelog --all" does not work correctly. It creates a CHANGELOG.rst file in the
testdirectory.I merged the CMakeLists.txt files from
serverandtestdirectories into a single file and placed it in the top-level directory of the repository. To compile the code in the test directory, the cmake variable "CLIENT_TEST_DEMO=ON" need to be set.Move the package.xml file from the server directory to the top-level directory of the repository.
At the same time, I change the package name to "persist_parameter_server" to keep it the same as ros/rosdistro#47829.